-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a file cache #186
Add a file cache #186
Conversation
self.document_map.get(&uri).unwrap() | ||
}, | ||
}; | ||
|
||
// TODO: This range seems to be off |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can ignore the contents of the file. I've found a good way of making Ante thread-safe while also allowing aliased mutability without lifetime annotations!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've determined this is because of larger unicode characters present in the file causing the internal byte index to be different than the character index.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice! Big improvements in the LS and in getting rid of the read file or panic function while displaying errors.
I think there are some changes we could make to reduce the cloning of files. Since I can't edit this PR myself though, I'll log the cases here, merge this, and make a follow up commit myself to reduce them to reduce your workload.
Thank you again! Do you have plans on what to work on for ante-ls next?
Well, one way to reduce the cost of the clones would be using an I think I'm going to try implementing hover next. |
let range = rope_range_to_lsp_range(loc.start.index..loc.end.index, &rope); | ||
let message = format!("{}", diagnostic.display(&cache)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it looks like this change on master was accidentally reverted.
Returning a |
After jfecher#186 all module paths should be absolute, and as such it's not necessary to prefix them with ./
* Implement named constructor * Fix llvm backend trying to use relative paths After #186 all module paths should be absolute, and as such it's not necessary to prefix them with ./
I've added a file cache to ModuleCache. That should allow for passing in memory buffers from the lsp to the compiler, and therefore updating diagnostics without having to wait for a file write. It also changes error messages to be relative paths from the root each module is in, and as such the expected errors for tests had to be updated.
Additionally, I've separated the check function into a frontend module that is reused in the language server like previously suggested.